Optimize the `ToHexString` #3566

nan01ab · 2024-11-04T17:12:39Z

Description

I tested some RPC interfaces and found that GC consumes a quite bit of CPU.

•Runtime Version: V 8.0.824.36612 (built on 17/7/2024 02:12:37)
•CLR Startup Flags: 8388611
•Total CPU Time: 34,098 msec
•Total GC CPU Time: 6,984 msec
•Total Allocs : 8,984.575 MB
•Number of Heaps: 1
•GC CPU MSec/MB Alloc : 0.777 MSec/MB
•Total GC Pause: 7,440.0 msec
•% Time paused for Garbage Collection: 27.6%
•% CPU Time spent Garbage Collecting: 20.5%
•Max GC Heap Size: 194.878 MB
•Peak Process Working Set: 311.857 MB
•Peak Virtual Memory Usage: 2,481,154.531 MB

And I found some memory allocations that can be optimized.

For example:
Th current impl of ToHexString:

StringBuilder can be pre-allocated.
AppendFormat will box the byte b and causes a/some memory allocation(s).

        StringBuilder sb = new();
        foreach (var b in bytes)
            sb.AppendFormat("{0:x2}", b);
        return sb.ToString();

Perf tests on 256B bytes:

BenchmarkDotNet v0.13.12, macOS Ventura 13.7 (22H123) [Darwin 22.6.0]
Intel Core i7-8750H CPU 2.20GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET SDK 8.0.203
  [Host]     : .NET 8.0.3 (8.0.324.11423), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.3 (8.0.324.11423), X64 RyuJIT AVX2


| Method                   | Mean        | Error     | StdDev    |
|------------------------- |------------:|----------:|----------:|
| ToHex_Optimized          |    339.0 ns |   6.70 ns |   7.71 ns |
| ToHex_Reversed_Optimized |    525.0 ns |  10.44 ns |   9.76 ns |
| ToHexSpan_Optimized      |    880.8 ns |  17.47 ns |  26.69 ns |
| ToHex_Legacy             | 10,236.3 ns | 172.80 ns | 161.63 ns |
| ToHex_Reversed_Legacy    | 10,445.8 ns | 185.36 ns | 206.03 ns |

Type of change

Optimization (the change is only an optimization)
Style (the change is only a code style for better maintenance or standard purpose)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

shargon · 2024-11-05T08:34:07Z

src/Neo.Extensions/ByteExtensions.cs

@@ -17,7 +17,7 @@ namespace Neo.Extensions
 {
    public static class ByteExtensions
    {
-        private static readonly char[] s_hexChars = "0123456789abcdef".ToCharArray();
+        private const string s_hexChars = "0123456789abcdef";


How can be const string faster than char string?

If access to the char of a string is faster than access to the char array, the benchmark was wrong

If access to the char of a string is faster than access to the char array, the benchmark was wrong

Const value may more friendly for compiler and JIT optimization.

If access to the char of a string is faster than access to the char array, the benchmark was wrong

UTF-8 string is slower, but this string just asc-ii, so it can be optimized.

https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/String.cs#L750-L753

Wrong benchmark, i'm sure

https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/String.cs#L750-L753

Wrong benchmark, i'm sure

Why this is slower, there are just two operations: an if check and a pointer add(a lea instruction on x86), and it has an Intrinsic attribute, so JIT can optimize it.

There are also a bound check(for IndexOutOfRangeException) and a pointer computation on array access.

https://learn.microsoft.com/en-us/dotnet/core/tools/dotnet-environment-variables#dotnet_jit-and-dotnet_gc
https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/jit/investigate-stress.md#jit-stress-debug-builds-only--debug-and-checked

This is the one you want
https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/jit/viewing-jit-dumps.md#disassembly-output

enable this on your computer and run dotnet in commandline to test JIT and GC outputs

DOTNET_JitDisasm={method-list} - output disassembly for the specified functions. E.g., DOTNET_JitDisasm=Main, DOTNET_JitDisasm=Main Test1 Test2, DOTNET_JitDisasm=* (for all functions).

vncoelho

I like your analysis.
I need 2 days to benchmark as well

shargon

I prefer char array, it will be faster, but if all @neo-project/core is ok, i'm ok

src/Neo.Cryptography.BLS12_381/Scalar.cs

cschuchardt88 · 2024-11-06T13:28:30Z

@shargon can we wait on merging this until #3503 please. It will be a pain to update that pr.

nan01ab · 2024-11-06T14:11:07Z

I prefer char array, it will be faster, but if all @neo-project/core is ok, i'm ok

I tested again.
const string is faster when ToHexString([]byte), char[] is faster when ToHexString(reverse = true), no difference when ToHexString(Span);

ToHexString([]byte) is used in most cases , so cost string is used(May more friendly for some compiler optimazation)

shargon · 2024-11-07T14:04:32Z

I prefer char array, it will be faster, but if all @neo-project/core is ok, i'm ok

I tested again. const string is faster when ToHexString([]byte), char[] is faster when ToHexString(reverse = true), no difference when ToHexString(Span);

ToHexString([]byte) is used in most cases , so cost string is used(May more friendly for some compiler optimazation)

this always will be slower than direct array access, but it's ok, just a ms

https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/String.cs#L750-L753

nan01ab and others added 2 commits November 5, 2024 00:55

fea: optimize ToHexString

3990c30

use char array instead string

ac15bd3

shargon previously approved these changes Nov 4, 2024

View reviewed changes

cschuchardt88 previously approved these changes Nov 4, 2024

View reviewed changes

cschuchardt88 added the Ready to Merge label Nov 4, 2024

1. const string is faster; 2. Scalar.ToString uses ToHexString

2bbb8fe

nan01ab dismissed stale reviews from cschuchardt88 and shargon via 2bbb8fe November 5, 2024 01:25

Jim8y previously approved these changes Nov 5, 2024

View reviewed changes

Jim8y requested review from shargon and cschuchardt88 November 5, 2024 02:00

Merge branch 'master' into fea.optimize-hex-string

b72430c

shargon reviewed Nov 5, 2024

View reviewed changes

Use StringBuilder

d89e678

shargon dismissed Jim8y’s stale review via d89e678 November 5, 2024 08:36

shargon added 2 commits November 5, 2024 09:42

Optimize capacity

d977dfc

Reduce changes

d8cb4d9

vncoelho reviewed Nov 6, 2024

View reviewed changes

NGDAdmin and others added 2 commits November 6, 2024 10:47

Merge branch 'master' into fea.optimize-hex-string

2e2277a

Merge branch 'master' into fea.optimize-hex-string

d1b17c8

shargon approved these changes Nov 6, 2024

View reviewed changes

Merge branch 'master' into fea.optimize-hex-string

dd04655

nan01ab commented Nov 6, 2024

View reviewed changes

src/Neo.Cryptography.BLS12_381/Scalar.cs Show resolved Hide resolved

Merge branch 'master' into fea.optimize-hex-string

6123f16

Jim8y approved these changes Nov 7, 2024

View reviewed changes

Jim8y added Blocked This issue can't be worked at the moment and removed Ready to Merge labels Nov 7, 2024

Merge branch 'master' into fea.optimize-hex-string

9105189

Jim8y added Ready to Merge and removed Blocked This issue can't be worked at the moment labels Nov 8, 2024

shargon merged commit 8b2c33b into neo-project:master Nov 8, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize the `ToHexString` #3566

Optimize the `ToHexString` #3566

nan01ab commented Nov 4, 2024

shargon Nov 5, 2024

shargon Nov 5, 2024

nan01ab Nov 5, 2024

nan01ab Nov 6, 2024

shargon Nov 7, 2024

nan01ab Nov 8, 2024 •

edited

Loading

cschuchardt88 Nov 8, 2024 •

edited

Loading

vncoelho left a comment

shargon left a comment •

edited

Loading

cschuchardt88 commented Nov 6, 2024 •

edited

Loading

nan01ab commented Nov 6, 2024 •

edited

Loading

shargon commented Nov 7, 2024

Optimize the ToHexString #3566

Optimize the ToHexString #3566

Conversation

nan01ab commented Nov 4, 2024

Description

Type of change

Checklist:

shargon Nov 5, 2024

Choose a reason for hiding this comment

shargon Nov 5, 2024

Choose a reason for hiding this comment

nan01ab Nov 5, 2024

Choose a reason for hiding this comment

nan01ab Nov 6, 2024

Choose a reason for hiding this comment

shargon Nov 7, 2024

Choose a reason for hiding this comment

nan01ab Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

cschuchardt88 Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

vncoelho left a comment

Choose a reason for hiding this comment

shargon left a comment • edited Loading

Choose a reason for hiding this comment

cschuchardt88 commented Nov 6, 2024 • edited Loading

nan01ab commented Nov 6, 2024 • edited Loading

shargon commented Nov 7, 2024

Optimize the `ToHexString` #3566

Optimize the `ToHexString` #3566

nan01ab Nov 8, 2024 •

edited

Loading

cschuchardt88 Nov 8, 2024 •

edited

Loading

shargon left a comment •

edited

Loading

cschuchardt88 commented Nov 6, 2024 •

edited

Loading

nan01ab commented Nov 6, 2024 •

edited

Loading